Fix names of error in test descriptions of Kernel#raise#1315
Fix names of error in test descriptions of Kernel#raise#1315eregon merged 1 commit intoruby:masterfrom
Conversation
This was probably a leftover of a copy-paste, these should check TypeError instead of ArgumentError.
|
|
||
| it "doesn't raise an ArgumentError when given cause is nil" do | ||
| it "doesn't raise a TypeError when given cause is nil" do | ||
| -> { raise "message", cause: nil }.should raise_error(RuntimeError, "message") |
There was a problem hiding this comment.
Hmm, shouldn't this be a TypeError here then?
There was a problem hiding this comment.
doesn't raise a TypeError, i.e. it succeeds but this is raise so succeeding means throwing an exception
There was a problem hiding this comment.
Oops, somehow completely blinded the "doesn't" part. Maybe the description would be better as "raises RuntimeError".
There was a problem hiding this comment.
It was written to contrast the test above it:
it "raises a TypeError when given cause is not an instance of Exception" do
-> { raise "message", cause: Object.new }.should raise_error(TypeError, "exception object expected")
end
it "doesn't raise a TypeError when given cause is nil" do
-> { raise "message", cause: nil }.should raise_error(RuntimeError, "message")
endWe're testing (speccing?) the different types that can be given as the cause argument. An object that is not nil and not an Exception is not allowed, so the cause argument causes the exception.
In the second test, we're passing a nil as cause argument, this is accepted and ignored, and now the raised exception is what we explicitly do with raise.
So both cases throw an exception, but it's for very different reasons, and the second one is the happy path.
Maybe something like "it accepts nil as a value for cause is accepted and ignored" would be a better description. And to put the icing on the cake, we should probably add a block to raise_error and verify e.cause.should be_nil
There was a problem hiding this comment.
A describe "the cause: argument" do would also help clarity & structure, and make it clearer those examples are related.
There was a problem hiding this comment.
Considering it's me who was confused, I will make a PR for this soon™
This was probably a leftover of a copy-paste, these should check TypeError instead of ArgumentError.